ci: fix e2e-sandbox by using the nebari-apps ArgoCD project, migrate to sandbox v3 - #76
Merged
Merged
Conversation
…to sandbox v3 The e2e-sandbox job has been red on main for several commits, in two distinct phases. The originally-reported failure was Keycloak never being installed by the sandbox platform stack. That was NIC v0.13.0 removing the existing-cluster + file:// GitOps combination the action's k3d path depended on, reached via the action's `nic-version: latest` default. action-nebari-sandbox v2.3.0 fixed it upstream by pinning NIC to v0.12.0, so the platform now comes up. What replaced it is ours. NIC locked the `default` ArgoCD AppProject down to a deny-all (empty sourceRepos and destinations) and added `nebari-apps` as the home for consumer applications. Our Application still declared `project: default`, so ArgoCD rejected it with two InvalidSpecErrors - the destination namespace and the file:// repo are both outside the project - and it never synced. The wait loop saw sync=Unknown health=Unknown for all 90 polls. Switching to `nebari-apps` matches the action's documented consumer pattern and its own test scenario. Also migrate off the k3d path, which v2 pins to NIC v0.12.0 permanently: - Bump to @V3, which deploys via NIC's local (kind) provider. v3 drops the `profile` input, as platform is now the only profile. - Pin `nic-version: v0.13.0` rather than tracking `latest`, so a NIC release cannot turn this job red without a deliberate bump - the same reasoning already applied to golangci-lint and kubeconform here. - Load the image with `kind load docker-image` instead of `k3d image import`. kind 0.32.0 is preinstalled on ubuntu-latest. - Drop the explicit teardown step. v3 destroys the deployment in its own post step, including on failure and cancellation. - Correct the dev-mode auth comment: v3 resolves *.nebari.local from inside the cluster, so the untrusted self-signed gateway CA is the only remaining reason real auth cannot be exercised here.
With the ArgoCD project fixed, the e2e job got all the way to the final gateway probe and failed there: curl to the gateway LoadBalancer IP timed out with no response at all. The gateway IP was 192.168.1.100, which is NIC's fallback MetalLB pool (192.168.1.100-192.168.1.110), not a derived one. kind puts its nodes on 172.18.0.0/16, so that address has no route from the runner and the connection never establishes. On the old k3d path the gateway IP was the node IP itself (172.18.0.2), directly routable, which is why this probe used to work. NIC does derive a pool from the kind Docker network, but caches it on the provider instance during Deploy, and InfraSettings only picks it up if Deploy populated that same instance first. Something in that chain does not hold, and the IPAddressPool silently gets the fallback - no warning is emitted, which rules out a derivation error and points at the derived value simply never being consulted. An explicitly configured address_pool takes precedence over both the cache and the fallback, so pin it. The pool is derived from the network kind will actually use rather than hardcoded, mirroring NIC's own formula (the .100-.110 range of the subnet's last /24 block), so it self-corrects if Docker picks a different subnet. The `kind` bridge network is created up front to fix that subnet, which kind then reuses. Supplying nic-config means the action skips its own config template, so this restates the platform defaults and pins cluster-name to match project_name, as the action requires for its outputs to resolve.
jbouder
added a commit
that referenced
this pull request
Aug 24, 2026
…truncation Rebased onto main, which brought #72, #73, #76, and #77. The rebase produced no git conflicts but two packages that did not compile, plus a migration collision that stopped the server booting. Fixes those, the codec data-loss bug, and the smaller items from the same review. Blockers: - Port the MCP write surface to the free-form body. `write.go` declared ten slot-named input fields against a `frames.Doc` that no longer has them. It now carries `body` and `template`: `body` because that is what a Frame's content is, and `template` because the reflective guard in `resources_test.go` asked for a decision and the answer is yes — without it `create_frame` cannot make a template at all, and an omitted-means-keep pointer stops `update_frame` de-listing one by accident. The guard now walks `frames.Doc` alone, since `SlotTable` is gone. - Renumber `005_frame_is_template.sql` to 006. main's `005_canonical_membership_ email.sql` claimed the same version, and goose rejects duplicates at provider construction, so no migration ran at all. Adds the missing `-- +goose Down`. `migrate_legacy_test.go` builds a schema as of 004 and did not carry the frames table, so 006's ALTER failed there as "no such table"; the fixture now carries every table a later migration touches, and asserts a pre-006 frame comes forward as not-a-template. - Match the frontmatter delimiter only at column 0. `TrimSpace(line) == "---"` let an indented `---` inside a YAML block scalar close the frontmatter, which truncated the document and dropped every field after it — including documents the exporter itself produced, where the error named a field the author never touched. Table-driven cases cover the block scalar in both positions, a `---` in the body, and a full round trip through the codec's own output. The unqualified "lossless" claims are replaced with the two normalizations that actually happen. Should-fix: - `frame-yaml.ts` claimed to mirror `legacy.go` and did not: no two-space continuation indent, and `.trim()` where Go trims newlines only. That reaches storage, since restoring a legacy version re-serializes the TypeScript render as canonical content — one rule with a nested list became three flat rules, permanently. The port is now faithful, and both sides are pinned to one shared fixture, `testdata/legacy-slots/`, compared whole rather than by substring. Substring assertions over single-line values are what let the drift through. Also: - Pin the legacy-vs-`body:` precedence, which was silent and untested, and stop `Parse` naming the unexported `frames.docYAML` in unknown-key errors that reach API clients unwrapped. The error now lists the recognized keys. - Cover `is_template` at the sqlite level: four SELECT lists and four Scan calls, none of them exercised by the service tests, which use `store.NewMemory()`. Asserts both the true and false case and the columns either side, so a scan-order mismatch fails. - Guard the empty version in `compose.go`'s `Inherits from:` line, matching `MarshalMarkdown`. - Give the one page-level heading that cannot use `PageHeader` the classes `PageHeader` exists to hold steady. - Rewrite the parts of AGENTS.md and the three design docs that described the deleted slot design as current. The MCP doc's §3.4 gets the struck-through supersede treatment #73 set the precedent for on that same file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #74.
Takes
e2e-sandboxfrom red to green. Two independent causes, plus the v3 migration the issue asked about.Cause 1: the reported symptom, already fixed upstream
Keycloak was never installed by the sandbox platform stack because NIC
v0.13.0removed the existing-cluster +file://GitOps combination the action's k3d path depended on, reached via the action'snic-version: latestdefault.action-nebari-sandboxv2.3.0(1369c90) fixed it by pinning NIC tov0.12.0.Answering the issue's first question directly:
@v2is still the k3d implementation, so there was no tag/implementation split.v3is where the kind migration landed.Cause 2: our Application was in a deny-all ArgoCD project
NIC commit
9547e51(nebari-infrastructure-core#458/#481) locked thedefaultAppProject down and addednebari-appsfor consumer apps:We still declared
project: default, so ArgoCD rejected the Application outright:The object existed but never synced -
sync=Unknown health=Unknownfor all 90 polls.nebari-appsis wildcard on bothsourceReposand destination namespace, and is what the action's README and its ownconsumer-app-via-gitopsscenario use. Verified present atv0.12.0as well as NICmain.Cause 3: v3's gateway IP was unroutable (upstream bug)
With the project fixed, the job reached the final gateway probe and timed out there with no response at all. The gateway IP was
192.168.1.100- NIC's fallback MetalLB pool, not a derived one. kind puts nodes on172.18.0.0/16, so that address has no route from the runner. On k3d the gateway IP was the node IP itself (172.18.0.2), directly routable, which is why this probe used to work.Root cause is an ordering bug in NIC.
pkg/nic/deploy.go:153computesInfraSettingsbeforeclusterProvider.Deployat line 193:For the
localprovider that assumption does not hold:InfraSettingsreadsp.metalLBPool, which onlyDeploypopulates, so the fallback always wins. Consistent with this, the "Could not derive MetalLB address pool" warning never fires - the derived value is computed correctly and then never consulted.An explicit
cluster.local.metallb.address_pooltakes precedence over both the cache and the fallback, so this PR pins it. The pool is derived from the network kind will actually use rather than hardcoded, mirroring NIC's own formula (the.100-.110range of the subnet's last/24), so it self-corrects if Docker picks a different subnet. Upstream issues are being filed against NIC and against the action, whose v3 test only regex-checks thatgateway-iplooks like an IPv4 address and never connects to it - which is how this shipped green.Changes
project: default->project: nebari-apps.@v3(NIC'slocal/kind provider). v3 drops theprofileinput, sinceplatformis now the only profile.nic-version: v0.13.0instead of v3'slatestdefault, addressing the issue's third suggestion. Same reasoning already applied togolangci-lintandkubeconformhere, and exactly the failure mode behind cause 1.nic-config, derived from thekindDocker network.k3d image import->kind load docker-image. kind 0.32.0 is preinstalled onubuntu-latest.*.nebari.localin-cluster, so the untrusted self-signed gateway CA is the only remaining blocker.Verification
All five checks green on run 32521445195. The e2e job did real work rather than passing vacuously:
The 200 response body is the actual nebari-frames app HTML, served through the gateway over TLS. Job runtime dropped from 19m50s (timing out) to 7m15s.
Follow-up worth filing
The sandbox now resolves
*.nebari.localin-cluster and publishes its CA as ConfigMapnebari-sandbox-cainkube-public, so this job could exercise real Keycloak auth instead ofdevModeby mounting that CA into the frames pod, covering the OIDC path currently skipped. Deliberately out of scope here.One unrelated leftover: the
chmod -R a+rX "${GITOPS_DIR}"in the deploy step is no longer needed (NIC >= v0.10.0 handles repo-server perms on commit). Harmless, so left alone rather than conflated with this fix.